chore(vmop): refactoring vmop restore/clone #1925
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eofff
force-pushed
the
chore/vmop/remove-restore-clone-complete-condition
branch
from
7bfa8e2 to
fac98f5
Compare
danilrwx
requested changes
Jan 27, 2026
images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go
Outdated
Show resolved
Hide resolved
images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go
Outdated
Show resolved
Hide resolved
images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go
Show resolved
Hide resolved
images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go
Show resolved
Hide resolved
images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go
Show resolved
Hide resolved
.../virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go
Outdated
Show resolved
Hide resolved
.../virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/exit_maintenance_step.go
Outdated
Show resolved
Hide resolved
images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go
Outdated
Show resolved
Hide resolved
images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go
Outdated
Show resolved
Hide resolved
danilrwx
reviewed
Jan 27, 2026
images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go
Outdated
Show resolved
Hide resolved
danilrwx
reviewed
Jan 27, 2026
images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go
Outdated
Show resolved
Hide resolved
eofff
changed the title
danilrwx
reviewed
Jan 28, 2026
...s/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step.go
Show resolved
Hide resolved
images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/final_step.go
Outdated
Show resolved
Hide resolved
danilrwx
previously approved these changes
Jan 28, 2026
eofff
force-pushed
the
chore/vmop/remove-restore-clone-complete-condition
branch
3 times, most recently
from
7995d93 to
8d47ebc
Compare
added 2 commits
January 29, 2026 11:39
Signed-off-by: Valeriy Khorunzhin <[email protected]> refactoring Signed-off-by: Valeriy Khorunzhin <[email protected]> fix Signed-off-by: Valeriy Khorunzhin <[email protected]> remove printf Signed-off-by: Valeriy Khorunzhin <[email protected]> fix e2e Signed-off-by: Valeriy Khorunzhin <[email protected]> total rewriting Signed-off-by: Valeriy Khorunzhin <[email protected]> fixes Signed-off-by: Valeriy Khorunzhin <[email protected]> tests Signed-off-by: Valeriy Khorunzhin <[email protected]> fix linter Signed-off-by: Valeriy Khorunzhin <[email protected]> fix linter 2 Signed-off-by: Valeriy Khorunzhin <[email protected]> final -> completed Signed-off-by: Valeriy Khorunzhin <[email protected]> resolve Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
eofff
force-pushed
the
chore/vmop/remove-restore-clone-complete-condition
branch
from
8d47ebc to
0bf2045
Compare
diafour
reviewed
Jan 29, 2026
| // Run execute until the operation is completed. | ||
| if svcOp.IsInProgress() { | ||
| return h.execute(ctx, vmop, svcOp) | ||
| if _, found := conditions.GetCondition(vmopcondition.TypeCompleted, vmop.Status.Conditions); found { |
Member
There was a problem hiding this comment.
According to comment, it should be !found or at least Status should be checked. Or change comment for more clarification.
diafour
reviewed
Jan 29, 2026
...virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go
Show resolved
Hide resolved
diafour
reviewed
Jan 29, 2026
images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/final_step_test.go
Outdated
Show resolved
Hide resolved
diafour
reviewed
Jan 29, 2026
Comment on lines
+76
to
+77
| Expect(phase1).To(Equal(phase2)) | ||
| Expect(phase2).To(Equal(phase3)) |
Member
There was a problem hiding this comment.
These asserts may generate unexpected results if phase in Equal 2 is not "Completed":
Something like "Test failed, expect Failed, got Completed"
diafour
reviewed
Jan 29, 2026
...irtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go
Outdated
Show resolved
Hide resolved
diafour
reviewed
Jan 29, 2026
images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go
Show resolved
Hide resolved
diafour
reviewed
Jan 29, 2026
.../virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step.go
Outdated
Show resolved
Hide resolved
diafour
reviewed
Jan 29, 2026
...irtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go
Outdated
Show resolved
Hide resolved
diafour
reviewed
Jan 29, 2026
...irtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go
Outdated
Show resolved
Hide resolved
added 3 commits
January 29, 2026 18:37
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Signed-off-by: Valeriy Khorunzhin <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Refactored VMOP Restore/Clone Controller
This PR refactors the VirtualMachineOperation controller for restore and clone operations. The implementation now uses a unified step-based execution flow, improving code maintainability and consistency between restore and clone operations.
Condition Changes
The RestoreCompleted and CloneCompleted conditions have been deprecated and removed. Both operations now use the unified Completed condition with appropriate reasons (ReasonRestoreInProgress, ReasonCloneInProgress, ReasonOperationCompleted, ReasonOperationFailed) to track operation progress and completion status.
Testing
Unit tests have been added to cover the refactored restore and clone step implementations.
Checklist